Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators #36030

Closed
wants to merge 1 commit into from

Conversation

vzaidman
Copy link
Contributor

@vzaidman vzaidman commented Feb 1, 2023

Summary:
Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators

For enums in the ts schema file such as:

export enum NumEnum {
  ONE = 1,
  TWO = 2,
}

This would export enums and the relevant Bridging to js and from js code to the spec H files such as:

#pragma mark - SampleTurboModuleCxxNumEnum

enum SampleTurboModuleCxxNumEnum { ONE, TWO };

template <>
struct Bridging<SampleTurboModuleCxxNumEnum> {
  static SampleTurboModuleCxxNumEnum fromJs(jsi::Runtime &rt, int32_t value) {

    if (value == 1) {
      return SampleTurboModuleCxxNumEnum::ONE;
    } else if (value == 2) {
      return SampleTurboModuleCxxNumEnum::TWO;
    } else {
      throw jsi::JSError(rt, "No appropriate enum member found for value");
    }
  }

  static jsi::Value toJs(jsi::Runtime &rt, SampleTurboModuleCxxNumEnum value) {
    if (value == SampleTurboModuleCxxNumEnum::ONE) {
      return bridging::toJs(rt, 1);
    } else if (value == SampleTurboModuleCxxNumEnum::TWO) {
      return bridging::toJs(rt, 2);
    } else {
      throw jsi::JSError(rt, "No appropriate enum member found for enum value");
    }
  }
};

That code would allow us to use these enums in the cxx files like this:

  NativeCxxModuleExampleCxxEnumInt getNumEnum(
      jsi::Runtime &rt,
      NativeCxxModuleExampleCxxEnumInt arg);

Changelog: [General] [Added] Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators

Differential Revision: D42884147

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Feb 1, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42884147

@analysis-bot
Copy link

analysis-bot commented Feb 1, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,480,550 +702
android hermes armeabi-v7a 7,801,453 +819
android hermes x86 8,956,331 +899
android hermes x86_64 8,814,119 +839
android jsc arm64-v8a 6,697,880 +720
android jsc armeabi-v7a 7,489,591 +832
android jsc x86 9,222,382 +926
android jsc x86_64 6,923,122 +850

Base commit: e168db4
Branch: main

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42884147

vzaidman added a commit to vzaidman/react-native that referenced this pull request Feb 1, 2023
…es generators (facebook#36030)

Summary:
Pull Request resolved: facebook#36030

Generate enum types in c++ turbo modules.

This would export enums to TM H files such as:
```
enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 };
```
And then use these enums in the TM schema such as:
```
public:
  virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0;
```
Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members.
```
public:
  virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0;
```

Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators

Differential Revision: D42884147

fbshipit-source-id: 18890e2f12c337d32c7e31a27604249027c1ec37
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42884147

vzaidman added a commit to vzaidman/react-native that referenced this pull request Feb 1, 2023
…es generators (facebook#36030)

Summary:
Pull Request resolved: facebook#36030

Generate enum types in c++ turbo modules.

This would export enums to TM H files such as:
```
enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 };
```
And then use these enums in the TM schema such as:
```
public:
  virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0;
```
Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members.
```
public:
  virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0;
```

Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators

Differential Revision: D42884147

fbshipit-source-id: 3245e4995501fd0436013bad1cb1a2d570618861
vzaidman added a commit to vzaidman/react-native that referenced this pull request Feb 1, 2023
…es generators (facebook#36030)

Summary:
Pull Request resolved: facebook#36030

Generate enum types in c++ turbo modules.

This would export enums to TM H files such as:
```
enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 };
```
And then use these enums in the TM schema such as:
```
public:
  virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0;
```
Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members.
```
public:
  virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0;
```

Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators

Differential Revision: D42884147

fbshipit-source-id: f51b943552a1af5b7ab4a2df94d62031e434da4b
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42884147

vzaidman added a commit to vzaidman/react-native that referenced this pull request Feb 1, 2023
…es generators (facebook#36030)

Summary:
Pull Request resolved: facebook#36030

Generate enum types in c++ turbo modules.

This would export enums to TM H files such as:
```
enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 };
```
And then use these enums in the TM schema such as:
```
public:
  virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0;
```
Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members.
```
public:
  virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0;
  virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0;
```

Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators

Differential Revision: D42884147

fbshipit-source-id: 730203057a60e586da3083930e643d7301a794b9
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42884147

…g/number in c++ turbo modules generators

Summary:
Generate enum types in c++ turbo modules.

For enums in the ts schema file such as:
```
export enum NumEnum {
  ONE = 1,
  TWO = 2,
}
```
This would export enums and the relevant Bridging to js and from js code to the spec H files such as:
```
#pragma mark - SampleTurboModuleCxxNumEnum

enum SampleTurboModuleCxxNumEnum { ONE, TWO };

template <>
struct Bridging<SampleTurboModuleCxxNumEnum> {
  static SampleTurboModuleCxxNumEnum fromJs(jsi::Runtime &rt, int32_t value) {

    if (value == 1) {
      return SampleTurboModuleCxxNumEnum::ONE;
    } else if (value == 2) {
      return SampleTurboModuleCxxNumEnum::TWO;
    } else {
      throw jsi::JSError(rt, "No appropriate enum member found for value");
    }
  }

  static jsi::Value toJs(jsi::Runtime &rt, SampleTurboModuleCxxNumEnum value) {
    if (value == SampleTurboModuleCxxNumEnum::ONE) {
      return bridging::toJs(rt, 1);
    } else if (value == SampleTurboModuleCxxNumEnum::TWO) {
      return bridging::toJs(rt, 2);
    } else {
      throw jsi::JSError(rt, "No appropriate enum member found for enum value");
    }
  }
};

```
That code would allow us to use these enums in the cxx files like this:
```
  NativeCxxModuleExampleCxxEnumInt getNumEnum(
      jsi::Runtime &rt,
      NativeCxxModuleExampleCxxEnumInt arg);
```

Changelog: [General] [Added] Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators

Reviewed By: christophpurrer

Differential Revision: D42884147

fbshipit-source-id: 83639022f59c18028ae16d8e2c491686e8867ab0
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42884147

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 13, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in ceb1d0d.

@vzaidman vzaidman deleted the export-D42884147 branch February 13, 2023 23:16
@vzaidman vzaidman changed the title Generate enum types instead of using string/number in c++ turbo modules generators Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators Feb 13, 2023
rshest added a commit to rshest/react-native that referenced this pull request Feb 14, 2023
…degen

Summary:
[Changelog][Internal]

The PR  facebook#36030 (diff D42884147 (facebook@ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules.

This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen.

This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members.

Some part of the change is the same as D42008724 (facebook@963e45a), but there are also some changes related to the types, that were required.

Differential Revision: D43292254

fbshipit-source-id: 50066dc89c495ba3cad23a26024ac485d03dc9d9
rshest added a commit to rshest/react-native that referenced this pull request Feb 14, 2023
…ceObserver

Summary:
[Changelog][Internal]

After D42884147 (facebook@ceb1d0d) (facebook#36030) had landed, we now can use enums as data types when bridging between JS and C++ TurboModules.

This diff takes advantage of this and converts the API to use the proper enums instead of int types that it was forced to use earlier.

NOTE: This also relies on additional change to codegen, D43292254, which is required in order for enums to work also as part of data structures.

Differential Revision: D43292282

fbshipit-source-id: fd271704ab60aec2b3ae5b260e76665ad50e206e
facebook-github-bot pushed a commit that referenced this pull request Feb 15, 2023
…degen (#36155)

Summary:
Pull Request resolved: #36155

[Changelog][Internal]

The PR  #36030 (diff D42884147 (ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules.

This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen.

This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members.

Some part of the change is the same as D42008724 (963e45a), but there are also some changes related to the types, that were required.

Reviewed By: christophpurrer

Differential Revision: D43292254

fbshipit-source-id: b2d6cf4a2d4d233b8cc403ecd02b5be16d5d91a7
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…g/number in c++ turbo modules generators (facebook#36030)

Summary:
Pull Request resolved: facebook#36030

Generate enum types in c++ turbo modules.

For enums in the ts schema file such as:
```
export enum NumEnum {
  ONE = 1,
  TWO = 2,
}
```
This would export enums and the relevant Bridging to js and from js code to the spec H files such as:
```
#pragma mark - SampleTurboModuleCxxNumEnum

enum SampleTurboModuleCxxNumEnum { ONE, TWO };

template <>
struct Bridging<SampleTurboModuleCxxNumEnum> {
  static SampleTurboModuleCxxNumEnum fromJs(jsi::Runtime &rt, int32_t value) {

    if (value == 1) {
      return SampleTurboModuleCxxNumEnum::ONE;
    } else if (value == 2) {
      return SampleTurboModuleCxxNumEnum::TWO;
    } else {
      throw jsi::JSError(rt, "No appropriate enum member found for value");
    }
  }

  static jsi::Value toJs(jsi::Runtime &rt, SampleTurboModuleCxxNumEnum value) {
    if (value == SampleTurboModuleCxxNumEnum::ONE) {
      return bridging::toJs(rt, 1);
    } else if (value == SampleTurboModuleCxxNumEnum::TWO) {
      return bridging::toJs(rt, 2);
    } else {
      throw jsi::JSError(rt, "No appropriate enum member found for enum value");
    }
  }
};

```
That code would allow us to use these enums in the cxx files like this:
```
  NativeCxxModuleExampleCxxEnumInt getNumEnum(
      jsi::Runtime &rt,
      NativeCxxModuleExampleCxxEnumInt arg);
```

Changelog: [General] [Added] Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators

Reviewed By: christophpurrer

Differential Revision: D42884147

fbshipit-source-id: d34d1fc7ba268b570821dc108444196f69a431b2
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…degen (facebook#36155)

Summary:
Pull Request resolved: facebook#36155

[Changelog][Internal]

The PR  facebook#36030 (diff D42884147 (facebook@ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules.

This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen.

This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members.

Some part of the change is the same as D42008724 (facebook@963e45a), but there are also some changes related to the types, that were required.

Reviewed By: christophpurrer

Differential Revision: D43292254

fbshipit-source-id: b2d6cf4a2d4d233b8cc403ecd02b5be16d5d91a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants